Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feat: supports optional dependencies #168

Merged
merged 2 commits into from
Nov 21, 2021
Merged

Conversation

SukkaW
Copy link
Contributor

@SukkaW SukkaW commented Nov 21, 2021

Fix #165, close #166.

Make terser, cssnano, uncss, etc. optional.

BREAKING CHANGES

Only merge it when it is about to bump a major version.


@maltsev What about creating a new branch v2? I'd like to introduce other breaking changes on top of this PR (like making purgecss default instead of uncss). With a new branch, we can only merge breaking changes into master when 2.0.0 is about to release.

@netlify
Copy link

netlify bot commented Nov 21, 2021

‼️ Deploy request for htmlnano rejected.

Name Link
🔨 Latest commit 20c4eae

Comment on lines +40 to +45
const optionalDependencies = {
minifyCss: ['cssnano', 'postcss'],
minifyJs: ['terser'],
minifyUrl: ['relateurl', 'srcset', 'terser'],
minifySvg: ['svgo'],
};
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

purgecss and uncss are not included here since only one of them is required when enabled.

Comment on lines +64 to +74
(optionalDependencies[moduleName] || []).forEach(dependency => {
try {
require(dependency);
} catch (e) {
if (e.code === 'MODULE_NOT_FOUND') {
console.warn(`You have to install "${dependency}" in order to use htmlnano's "${moduleName}" module`);
} else {
throw e;
}
}
});
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Try to resolve the required dependencies before HTML actually being minified. With Node.js's require cache it won't add any performance overhead when those dependencies are required again.

Comment on lines +84 to +88
htmlnano.getRequiredOptionalDependencies = function (optionsRun, presetRun) {
const [options] = loadConfig(optionsRun, presetRun);

return [...new Set(Object.keys(options).filter(moduleName => options[moduleName]).map(moduleName => optionalDependencies[moduleName]).flat())];
};
Copy link
Contributor Author

@SukkaW SukkaW Nov 21, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

getRequiredOptionalDependencies API is designed for other libraries that utilize htmlnano.

For example, parcel-bundler has a plugin @parcel/optimizer-htmlnano built on top of htmlnano. This API should enable the parcel to install those optional dependencies automatically.

Ideas and opinions from parcel developers (@devongovett @mischnic) would be appreciated.

@maltsev maltsev changed the base branch from master to v2 November 21, 2021 19:22
@maltsev
Copy link
Member

maltsev commented Nov 21, 2021

What about creating a new branch v2?

Sure. Good idea! I created it and going to merge your PR to v2.

@maltsev maltsev merged commit 08714f3 into posthtml:v2 Nov 21, 2021
This was referenced Nov 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove uncss?
2 participants